-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[SDKS-7830] flag sets #44
Conversation
CHANGES.txt
Outdated
@@ -1,5 +1,6 @@ | |||
2.0.0 (Dec XX, 2023) | |||
- Updated the minimum Angular version to match Angular's support up to date. Breaking change version is regarding the Angular minimum version update, there are no breaking changes to Split's plugin API or functionality itself. | |||
- Updated @splitsoftware/splitio-browserjs package to version 0.12.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Update to version 0.13.0
@@ -1,5 +1,6 @@ | |||
2.0.0 (Dec XX, 2023) | |||
- Updated the minimum Angular version to match Angular's support up to date. Breaking change version is regarding the Angular minimum version update, there are no breaking changes to Split's plugin API or functionality itself. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would check other CHANGES.txt files if there is a format for breaking change, something like: - BREAKING CHANGE: Updated ...
.
@@ -1,5 +1,6 @@ | |||
2.0.0 (Dec XX, 2023) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't forget to add an item for flag sets (either here or in a different PR). Also other updates like defaultTreatment
in SplitView, etc.
* @param {Attributes=} attributes - An object of type Attributes defining the attributes for the given key. | ||
* @returns {Treatments} - The treatments object map. | ||
*/ | ||
getTreatmentsByFlagSet(key: SplitIO.SplitKey, flagSetName: string, attributes?: SplitIO.Attributes | undefined): SplitIO.Treatments; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
attributes?: SplitIO.Attributes | undefined
-> attributes?: SplitIO.Attributes
(undefined is redundant if using ?
).
Check other method signatures too.
@@ -33,6 +33,10 @@ export const CONTROL_CLIENT = { | |||
}); | |||
return result; | |||
}, | |||
getTreatmentsByFlagSet: () => { return []; }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should return {}
. Some test should be validating it.
TDD: a test should be failing, and after the fix the test should pass.
@@ -6,7 +6,9 @@ export const mockedFeatureFlagView = [ | |||
'name': | |||
'test_split', | |||
'trafficType': 'localhost', | |||
'treatments': ['on'] | |||
'treatments': ['on'], | |||
'defaultTreatment': 'control', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is just a mock, but defaultTreatment
cannot be 'control'. It is one of the items in treatments
list.
…pes. update changes file.
Angular SDK plugin
What did you accomplish?
splitio-browserjs
to0.12.0
How do we test the changes introduced in this PR?
Extra Notes